Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove $ symbol from some commands #5979

Merged
merged 2 commits into from
Aug 3, 2022
Merged

Remove $ symbol from some commands #5979

merged 2 commits into from
Aug 3, 2022

Conversation

cdrage
Copy link
Member

@cdrage cdrage commented Jul 27, 2022

What type of PR is this:

/kind documentation

What does this PR do / why we need it:

This PR does the following:

  • Removes all $ from commands that should be "copied-and-pasted"
  • Separates all console output to a separation section.

Notes:

  • "Details" markdown feature would of been used, but Docusaurus does not
    support
    within a .

Which issue(s) this PR fixes:

Fixes #5844

PR acceptance criteria:

  • Documentation

How to test changes / Special notes to the reviewer:

Signed-off-by: Charlie Drage [email protected]

@cdrage
Remove $ symbol from some commands
4807ca8 
<!--
Thank you for opening a PR! Here are some things you need to know before submitting:

1. Please read our developer guideline: https://github.com/redhat-developer/odo/wiki/Dev:-odo-Dev-Guidelines
2. Label this PR accordingly with the '/kind' line
3. Ensure you have written and ran the appropriate tests: https://github.com/redhat-developer/odo/wiki/Dev:-Writing-and-running-tests
4. Read how we approve and LGTM each PR: https://github.com/redhat-developer/odo/wiki/Pull-Requests:-Review-guideline

Documentation:

If you are pushing a change to documentation, please read: https://github.com/redhat-developer/odo/wiki/Documentation:-Contributing
-->

**What type of PR is this:**

<!--
Add one of the following kinds:
/kind bug
/kind feature
/kind cleanup
/kind tests

Feel free to use other [labels](https://github.com/redhat-developer/odo/labels) as needed. However one of the above labels must be present or the PR will not be reviewed. This instruction is for reviewers as well.
-->
/kind documentation

**What does this PR do / why we need it:**

This PR does the following:
* Removes all `$` from commands that should be "copied-and-pasted"
* Separates all console output to a separation section.

Notes:
* "Details" markdown feature would of been used, but Docusaurus does not
  support <details> within a <tab>.

**Which issue(s) this PR fixes:**
<!--
Specifying the issue will automatically close it when this PR is merged
-->

Fixes redhat-developer#5844

**PR acceptance criteria:**

- [X] Documentation

**How to test changes / Special notes to the reviewer:**

Signed-off-by: Charlie Drage <[email protected]>
@netlify
Copy link

netlify bot commented Jul 27, 2022 •
edited
Loading

✅ Deploy Preview for odo-docusaurus-preview ready!

Name Link
🔨 Latest commit b7f5f26
🔍 Latest deploy log https://app.netlify.com/sites/odo-docusaurus-preview/deploys/62e913c943a1450009e422df
😎 Deploy Preview https://deploy-preview-5979--odo-docusaurus-preview.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@openshift-ci openshift-ci bot requested review from dharmit and feloy July 27, 2022 00:14
@odo-robot
Copy link

odo-robot bot commented Jul 27, 2022 •
edited
Loading

Unit Tests on commit c318c23 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Jul 27, 2022 •
edited
Loading

Windows Tests (OCP) on commit c318c23 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Jul 27, 2022 •
edited
Loading

Kubernetes Tests on commit c318c23 finished successfully.
View logs: TXT HTML

@odo-robot
Copy link

odo-robot bot commented Jul 27, 2022 •
edited
Loading

OpenShift Tests on commit c318c23 finished successfully.
View logs: TXT HTML

valaparthvi
valaparthvi suggested changes Jul 27, 2022
View reviewed changes
docs/website/docs/command-reference/create-namespace.md Outdated
Comment on lines 13 to 16

Example -
```shell
odo create namespace mynamespace
✓ Namespace "mynamespace" is ready for use
✓ New namespace created and now using namespace: mynamespace
```
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about this. Currently, both the section about command and output seem unrelated to each other.
I would rather keep this as the following -

To create a namespace you can run odo create namespace <name>.
Example -

$ odo create namespace mynamespace
 ✓  Namespace "mynamespace" is ready for use
 ✓  New namespace created and now using namespace: mynamespace

Same comment for all the changes.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1. I was scrolling through odo describe binding and it felt weird to me. Attaching screenshot:

image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that's intentional. But @valaparthvi has a good point and I'll update it so that it includes the commands for the examples.

The intention was to keep it separate so we don't have to add a "Example output:" each time to each command..

@odo-robot
Copy link

odo-robot bot commented Jul 27, 2022 •
edited
Loading

Validate Tests on commit c318c23 finished successfully.
View logs: TXT HTML

@cdrage
Copy link
Member Author

cdrage commented Jul 28, 2022

Updated the PR based on the review 👍

@dharmit
Copy link
Member

dharmit commented Jul 29, 2022

/approve

@openshift-ci
Copy link

openshift-ci bot commented Jul 29, 2022

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: dharmit

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@openshift-ci openshift-ci bot added the approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. label Jul 29, 2022
valaparthvi
valaparthvi suggested changes Jul 29, 2022
View reviewed changes
Copy link
Contributor

@valaparthvi valaparthvi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is probably out of scope for this PR, but I still think it's relevant to it.
There are a few docs where the code block also includes a comment, I don't like the idea of copying the comment when I copy the code, and I was wondering if we can remove the comment out of the code block from the following docs:

I also noticed that there is an inconsistency in these command-reference docs. Some commands show a sample output along with them, while some don't. I'm wondering if you can fix that as well.

odo logs command does not have copy command at all like other docs, do you think we can fix that?

I understand these are a lot of changes, and some unrelated, so I wouldn't mind if they're taken up in a separate PR.

docs/website/docs/command-reference/describe-binding.md Outdated
odo describe binding --name <component_name>
```

$ odo describe binding --name <component_name>
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not code blocked. Can you fix it?

@sonarqubecloud
Copy link

sonarqubecloud bot commented Aug 2, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@cdrage
Copy link
Member Author

cdrage commented Aug 2, 2022

The comments in the code blocks IMO are okay, but we can leave that to a cabal topic. It's similar to kubectl / kubernetes.io documentation on their code copy blocks. Makes sense to keep the comments "within" the copy and paste code.

With regards to some commands being inconsistent in example output, you are totally right, that's outside the scope of the PR (that you mentioned).

The docs team plans to do a refactor of our documentation so we'll have a lot more consistency then for examples!

@valaparthvi
Copy link
Contributor

/lgtm
Thanks for your work on this, Charlie.

@openshift-ci openshift-ci bot added the lgtm Indicates that a PR is ready to be merged. Required by Prow. label Aug 3, 2022
@valaparthvi valaparthvi mentioned this pull request Aug 3, 2022
Closed
4 tasks
@openshift-ci openshift-ci bot merged commit 0a89798 into redhat-developer:main Aug 3, 2022
cdrage added a commit to cdrage/odo that referenced this pull request Aug 31, 2022
@cdrage
Remove $ symbol from some commands (redhat-developer#5979)
45226fb 
* Remove $ symbol from some commands

<!--
Thank you for opening a PR! Here are some things you need to know before submitting:

1. Please read our developer guideline: https://github.com/redhat-developer/odo/wiki/Dev:-odo-Dev-Guidelines
2. Label this PR accordingly with the '/kind' line
3. Ensure you have written and ran the appropriate tests: https://github.com/redhat-developer/odo/wiki/Dev:-Writing-and-running-tests
4. Read how we approve and LGTM each PR: https://github.com/redhat-developer/odo/wiki/Pull-Requests:-Review-guideline

Documentation:

If you are pushing a change to documentation, please read: https://github.com/redhat-developer/odo/wiki/Documentation:-Contributing
-->

**What type of PR is this:**

<!--
Add one of the following kinds:
/kind bug
/kind feature
/kind cleanup
/kind tests

Feel free to use other [labels](https://github.com/redhat-developer/odo/labels) as needed. However one of the above labels must be present or the PR will not be reviewed. This instruction is for reviewers as well.
-->
/kind documentation

**What does this PR do / why we need it:**

This PR does the following:
* Removes all `$` from commands that should be "copied-and-pasted"
* Separates all console output to a separation section.

Notes:
* "Details" markdown feature would of been used, but Docusaurus does not
  support <details> within a <tab>.

**Which issue(s) this PR fixes:**
<!--
Specifying the issue will automatically close it when this PR is merged
-->

Fixes redhat-developer#5844

**PR acceptance criteria:**

- [X] Documentation

**How to test changes / Special notes to the reviewer:**

Signed-off-by: Charlie Drage <[email protected]>

* fix based on review
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@valaparthvi valaparthvi valaparthvi requested changes

@dharmit dharmit Awaiting requested review from dharmit

@feloy feloy Awaiting requested review from feloy

Assignees

@valaparthvi valaparthvi

Labels
approved Indicates a PR has been approved by an approver from all required OWNERS files. Required by Prow. lgtm Indicates that a PR is ready to be merged. Required by Prow.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Remove $ from console output on documentation
3 participants
@cdrage @dharmit @valaparthvi